Skip to content

Conversation

@catalyst17
Copy link
Contributor

@catalyst17 catalyst17 commented Mar 12, 2025

TL;DR

Added token_type field to the HolderModel struct and included it in the serialization function.

We need it to properly detect token types for NFTs handlers, specifically for further metadata resolutions.

What changed?

  • Added a new TokenType field to the HolderModel struct with JSON tag token_type and ClickHouse tag token_type
  • Updated the serializeHolder function to populate the new TokenType field from the holder.TokenType value

How to test?

  1. Make a request to an endpoint that returns token holder data
  2. Verify that the response now includes the token_type field in the JSON output
  3. Check that the value matches the expected token type for the given token

Why make this change?

This change enhances the token holder API response by including the token type information, which allows clients to distinguish between different token standards (e.g., ERC-20, ERC-721, ERC-1155) without having to make additional queries. This provides more complete information about tokens in a single response.

Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@catalyst17 catalyst17 requested review from a team and vernonjohnson March 12, 2025 18:02
@catalyst17 catalyst17 marked this pull request as ready for review March 12, 2025 18:02
@catalyst17 catalyst17 force-pushed the feat/add-token-type-to-holders-response branch from ad1c9e4 to 7fa0d4c Compare March 12, 2025 18:43
Copy link
Contributor

@vernonjohnson vernonjohnson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. just double check the failing test

Copy link
Contributor Author

lgtm. just double check the failing test

hm weird, not getting it locally

@catalyst17 catalyst17 force-pushed the feat/add-token-type-to-holders-response branch from 7fa0d4c to a4c676b Compare March 13, 2025 00:32
@catalyst17 catalyst17 merged commit d532f55 into main Mar 13, 2025
5 checks passed
@catalyst17 catalyst17 deleted the feat/add-token-type-to-holders-response branch March 13, 2025 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants